Skip to content

Add PHPUnit workflow, fix path handling bug, REUSE/php-cs compliance#722

Open
Copilot wants to merge 7 commits intomasterfrom
copilot/add-string-concat-method
Open

Add PHPUnit workflow, fix path handling bug, REUSE/php-cs compliance#722
Copilot wants to merge 7 commits intomasterfrom
copilot/add-string-concat-method

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 25, 2026

Adds a missing PHPUnit GitHub Actions workflow, fixes a fragile path extraction pattern inherited from issue #711, and resolves REUSE/php-cs CI failures.

PHPUnit workflow

  • Added .github/workflows/phpunit.yml running composer run test:unit against PHP 8.2 / 8.3 / 8.4 on PR and push

Path handling fix (moveWithExclusions)

createBackup() had already been fixed for the explode()-splits-on-all-occurrences bug, but moveWithExclusions() had the identical pattern:

// Before — splits on every occurrence of $dataLocation in $path
// e.g. storage path ending in "nextcloud/" + file named "nextcloud.html" → wrong
$fileName = explode($dataLocation, $path)[1];

// After — always strips exactly the right prefix
$fileName = substr($path, strlen($dataLocation));

Regression tests for issue #711

  • testRelativePathExtractionForIssue711() — pure logic test proving the old explode() returns a directory path instead of the file path for the exact bug-report scenario
  • testCreateBackupCopiesNestedFilesCorrectly() — integration test: creates core/doc/admin/…/nextcloud.html, runs createBackup(), asserts the file is copied to the correct nested path with no double slashes

REUSE compliance

vendor-bin/phpunit/** was not covered by any annotation in REUSE.toml. Added it to the existing block that covers the other vendor-bin tool directories.

php-cs

Reformatted string concatenation in tests/unit/UpdaterTest.php from trailing-dot to leading-dot style as required by the project's php-cs-fixer config.


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI and others added 2 commits March 25, 2026 06:36
… createBackup()

Co-authored-by: skjnldsv <14975046+skjnldsv@users.noreply.github.com>
Agent-Logs-Url: https://github.com/nextcloud/updater/sessions/5f2df1e7-e17a-4242-b4c3-91a831b9048d
…cloud/privacy pattern

Co-authored-by: skjnldsv <14975046+skjnldsv@users.noreply.github.com>
Agent-Logs-Url: https://github.com/nextcloud/updater/sessions/5f2df1e7-e17a-4242-b4c3-91a831b9048d
Copilot AI changed the title [WIP] Add string concat method for nextcloudDir concatenations feat: add buildPath() helper and fix double-slash bug in createBackup() Mar 25, 2026
Copilot AI requested a review from skjnldsv March 25, 2026 06:47
Copilot AI and others added 2 commits March 25, 2026 06:53
Copilot AI changed the title feat: add buildPath() helper and fix double-slash bug in createBackup() fix: centralise path building, fix explode-based double-slash bug (issue #711), add PHPUnit Mar 25, 2026
@skjnldsv skjnldsv requested a review from blizzz March 25, 2026 08:16
@skjnldsv skjnldsv marked this pull request as ready for review March 25, 2026 08:16
@skjnldsv skjnldsv requested a review from Altahrim as a code owner March 25, 2026 08:16
Copilot AI changed the title fix: centralise path building, fix explode-based double-slash bug (issue #711), add PHPUnit Add PHPUnit workflow, fix path handling bug, REUSE/php-cs compliance Mar 25, 2026
@Keeper-of-the-Keys
Copy link
Copy Markdown

Hey @skjnldsv
The substr based approach to #711/#720 should also work, my only gripe (probably overly paranoid) with it is that it is (as currently implemented) just chopping of a set amount of characters without verifying that they actually are what we seek to remove.

@Keeper-of-the-Keys
Copy link
Copy Markdown

As a total aside - how wise is it to roll so many issues into 1 PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants